@hubert-reinterpretcast What do you think about merging this PR to the release branch?

Uhm - this looks pretty big and seems like something that can easily break certain build configurations since it doesn't seem to touch only AIX. Is this in main without any issues? Does it really NEED to be merged for the release branch at this point?

Uhm - this looks pretty big and seems like something that can easily break certain build configurations since it doesn't seem to touch only AIX

Agreed that this looks big and scary, but it's a purely mechanical change, that is a no-op for most targets. I'll add a long form rational at the end of the comment about why I don't think the patch effects anyone but AIX to keep my answers brief.

Is this in main without any issues?

Yes, these patches have been in main for several weeks at this point with no reported issues.

Does it really NEED to be merged for the release branch at this point?

It would help us out for the point releases. Without this patch, we're unable to build on AIX with CMake from our package manager (4.0). We can manually downgrade if we're unwilling to merge this, but it's a bit of a pain.

Rationale about why the patch doesn't affect targets besides AIX

We quote the string AIX and variable expansions which might expand to string AIX (i.e. CMAKE_SYSTEM_NAME), so that we do the intent string comparison. If not quoted the if will expand the string if it happens to match a variable name (which AIX does in CMake 4.0+).

This has an effect only if CMAKE_SYSTEM_NAME (https://cmake.org/cmake/help/latest/variable/CMAKE_SYSTEM_NAME.html) expands to something which is a CMake variable (https://cmake.org/cmake/help/latest/manual/cmake-variables.7.html#variables-that-describe-the-system)

Intersecting the two list gives me the following list of affect targets:

AIX
CYGWIN
MSYS
WASI

Of those targets, only CYGWIN appears in the lines affected by the patch, and it's already using a variable check (i.e. it checks CYGWIN) not a string comparison to CMAKE_SYSTEM_NAME, so it's unaffected.

Uhm - this looks pretty big and seems like something that can easily break certain build configurations since it doesn't seem to touch only AIX

Agreed that this looks big and scary, but it's a purely mechanical change, that is a no-op for most targets. I'll add a long form rational at the end of the comment about why I don't think the patch effects anyone but AIX to keep my answers brief.

Is this in main without any issues?

Yes, these patches have been in main for several weeks at this point with no reported issues.

Does it really NEED to be merged for the release branch at this point?

It would help us out for the point releases. Without this patch, we're unable to build on AIX with CMake from our package manager (4.0). We can manually downgrade if we're unwilling to merge this, but it's a bit of a pain.

Rationale about why the patch doesn't affect targets besides AIX

We quote the string AIX and variable expansions which might expand to string AIX (i.e. CMAKE_SYSTEM_NAME), so that we do the intent string comparison. If not quoted the if will expand the string if it happens to match a variable name (which AIX does in CMake 4.0+).

This has an effect only if CMAKE_SYSTEM_NAME (https://cmake.org/cmake/help/latest/variable/CMAKE_SYSTEM_NAME.html) expands to something which is a CMake variable (https://cmake.org/cmake/help/latest/manual/cmake-variables.7.html#variables-that-describe-the-system)

Intersecting the two list gives me the following list of affect targets:

AIX
CYGWIN
MSYS
WASI

Of those targets, only CYGWIN appears in the lines affected by the patch, and it's already using a variable check (i.e. it checks CYGWIN) not a string comparison to CMAKE_SYSTEM_NAME, so it's unaffected.

Hi @tru! Any thoughts regarding @daltenty's response on this backport?

I still feel pretty sketchy about this one. It seems large and could affect other platforms than AIX since it's changing the build system in pretty large ways.

I would be much more comfortable if we just drop this for the release branch and make sure it's working well in main before the next release. The main problem I have here is that there is also no maintainer for the CMake scripts in whole, I guess maybe @petrhosek would be the closest.

I understand it could be a bit of a pain for the AIX packages - but I wonder if it's not worth a little pain for a platform that ... frankly isn't that mainstream in order to avoid any potential issues with other platforms. I understand that it's not a blocker really, just annoying so that gives me more confidence to reject this.